✨Add an option to control initial rollout of HelmReleaseProxies #443

chaitanyakolluru · 2025-10-01T01:44:47Z

What this PR does / why we need it:
We do not have a way to control the initial rollout of HelmReleaseProxies for every matching cluster as part of a HelmChartProxy. This change introduces a new field to HelmChartProxy spec rolloutStepSize, which can be used to stagger the initial rollout of HelmReleaseProxies based on already existing HelmReleaseProxies's ready status.

This change will allow for a controlled rollout strategy that comes into place only during initial rollout and will not impact other options for ReconcileStrategy. A new status condition HelmReleaseProxiesRolloutCompleted is added to denote initial rollout status of HelmReleaseProxies and is included when determining overall summary Ready status condition on HelmChartProxy resources. For HelmChartProxies not using the rollout option, HelmReleaseProxiesRolloutCompleted status condition is added to the resource with a reason of HelmReleaseProxiesRolloutUndefined and a message that mentions not using the rollout option.

Below are two HelmChartProxy resource status conditions, one with and the other without rollout as an option:

Without rolloutStepSize:

[
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "Ready"
  },
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "HelmReleaseProxiesReady"
  },
  {
    "lastTransitionTime": "xxx",
    "message": "HelmChartProxy does not use rollout step",
    "reason": "HelmReleaseProxiesRolloutUndefined",
    "severity": "Info",
    "status": "True",
    "type": "HelmReleaseProxiesRolloutCompleted"
  },
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "HelmReleaseProxySpecsUpToDate"
  }
]

With rolloutStepSize:

[
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "Ready"
  },
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "HelmReleaseProxiesReady"
  },
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "HelmReleaseProxiesRolloutCompleted"
  },
  {
    "lastTransitionTime": "xxx",
    "status": "True",
    "type": "HelmReleaseProxySpecsUpToDate"
  }
]

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Signed-off-by: Chaitanya Kolluru <[email protected]>

linux-foundation-easycla · 2025-10-01T01:44:53Z

The committers listed above are authorized under a signed CLA.

✅ login: chaitanyakolluru / name: Chaitanya Kolluru (8d0c3a3)

k8s-ci-robot · 2025-10-01T01:44:56Z

Welcome @chaitanyakolluru!

It looks like this is your first PR to kubernetes-sigs/cluster-api-addon-provider-helm 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-addon-provider-helm has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-10-01T01:44:57Z

Hi @chaitanyakolluru. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mboersma · 2025-10-01T15:36:31Z

/ok-to-test

Signed-off-by: Chaitanya Kolluru <[email protected]>

dmvolod · 2025-10-01T20:18:49Z

api/v1alpha1/helmchartproxy_types.go

 	c.Status.Conditions = conditions
 }

+func (c *HelmChartProxy) GetHelmReleaseProxyReadyCondition() *clusterv1.Condition {


This kind of function already implemented in Cluster API conditions package. Please use it instead of create.

removed the fn and used Get() from cluster-api/util/conditions. Thanks.

…er-api's condition package. Signed-off-by: Chaitanya Kolluru <[email protected]>

dmvolod · 2025-10-01T20:32:50Z

controllers/helmchartproxy/helmchartproxy_controller.go

+			if err != nil {
+				return err
+			}
+			for _, hrpRltMeta := range clusterRolloutMeta {


Please note that the objects in the list are in a non-deterministic order, and processing them in batches during different reconciliation cycles may lead to unpredictable results. Are you sure this will be handled correctly?

Introduced orderliness on rollout meta before processing to prevent inconsistencies with different reconciliations over a list of matching clusters. Thanks for bringing this up.

dmvolod · 2025-10-01T20:42:20Z

controllers/helmchartproxy/helmchartproxy_controller.go

+			hrpReadyCond := conditions.Get(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition)
+			// If HelmReleaseProxiesReadyCondition is false, reconcile existing
+			// HelmReleaseProxies and exit.
+			if hrpReadyCond != nil && (hrpReadyCond.Status == corev1.ConditionFalse) {


Can you utilize conditions.IsTrue() or conditions.IsFalse() here and later in the code.

Used conditions.IsFalse() when checking for false status on the condition. I don't see other instance where conditions.IsTrue() can be used.

dmvolod · 2025-10-01T20:45:06Z

controllers/helmchartproxy/helmchartproxy_controller.go

+				if hrpRltMeta.hrpExists {
+					continue
+				}
+
+				// Don't reconcile if the Cluster is being deleted
+				if !hrpRltMeta.cluster.DeletionTimestamp.IsZero() {
+					continue
+				}
+
+				err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster)
+				if err != nil {
+					return err
+				}


There are very similar code to the code in lines 244-255, except condition in hrpRltMeta.hrpExists. You can wrap it with function (probably anonymous) and use it here and upper.

Moved cluster deletion timestamp check to within the reconcileForCluster() to make those sections look cleaner. Let me know if this works for you.

Signed-off-by: Chaitanya Kolluru <[email protected]>

…roxies Signed-off-by: Chaitanya Kolluru <[email protected]>

Signed-off-by: Chaitanya Kolluru <[email protected]>

dmvolod · 2025-10-07T06:52:01Z

api/v1alpha1/helmchartproxy_types.go

+	// creating HelmReleaseProxy resources for all matching clusters.
+	// e.g. an int (5) or percentage of count of total matching clusters (25%)
+	// +optional
+	RolloutStepSize *intstr.IntOrString `json:"rolloutStepSize,omitempty"`


Is it possible and currently processed changes to this field during recomcile, i.e. reset condition in case of reduce or increase it value?

It depends on the list of matched clusters per the clusterSelector field in the spec. Once all HelmReleaseProxies have been spawned during initial rollout, condition HelmReleaseProxiesRolloutCompletedCondition is marked True and upon changes to either the clusterSelector or the list of clusters being managed, the next reconcile will notice the difference in count of clusters matched to the HelmReleaseProxies, sets condition HelmReleaseProxiesRolloutCompletedCondition to False and rolls them out according to the rolloutStepSize defined. Hope that answers the question.

k8s-ci-robot · 2025-10-07T18:03:51Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chaitanyakolluru, dmvolod
Once this PR has been reviewed and has the lgtm label, please assign mboersma for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chaitanyakolluru · 2025-10-09T16:19:26Z

Could I get some eyes on this MR please. @mboersma @jackfrancis.

jackfrancis · 2025-10-10T21:35:30Z

controllers/helmchartproxy/helmchartproxy_controller_phases.go


 // reconcileForCluster will create or update a HelmReleaseProxy for the given cluster.
 func (r *HelmChartProxyReconciler) reconcileForCluster(ctx context.Context, helmChartProxy *addonsv1alpha1.HelmChartProxy, cluster clusterv1.Cluster) error {
+	// Don't reconcile if the Cluster is being deleted


Do we need this change in this PR?

Made this change as part of minimizing code reuse and moved the cluster deletion timestamp check to within the method instead.

#443 (comment)

jackfrancis · 2025-10-10T21:37:33Z

controllers/helmchartproxy/helmchartproxy_controller.go

 	return nil
 }
+
+// getNnStringFor to retrieve the namespaced name as a string.


nit: prefer the more verbose getNamespacedNameStringFor

updated name to getNamespacedNameStringFor

Signed-off-by: Chaitanya Kolluru <[email protected]>

chaitanyakolluru · 2025-10-16T13:38:40Z

@dmvolod @mboersma @jackfrancis made a couple of updates to how it decides to rollout the next batch of HelmReleaseProxies. Please take a look when you get a chance. Thanks.

k8s-ci-robot · 2025-11-03T17:13:26Z

@chaitanyakolluru: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-addon-provider-helm-e2e	`8d0c3a3`	link	true	`/test pull-cluster-api-addon-provider-helm-e2e`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

chaitanyakolluru · 2025-11-03T23:53:12Z

Closed in favor of #452

chaitanyakolluru added 2 commits September 30, 2025 20:12

adding rollingStepSize to helm chart proxy spec

bf381e6

Signed-off-by: Chaitanya Kolluru <[email protected]>

implementation and unit tests

95c6dcf

Signed-off-by: Chaitanya Kolluru <[email protected]>

k8s-ci-robot requested review from jackfrancis and mboersma October 1, 2025 01:44

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Oct 1, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 1, 2025

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Oct 1, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 1, 2025

chaitanyakolluru added 2 commits October 1, 2025 10:56

fixing reason names. updating condition message

725e3cb

Signed-off-by: Chaitanya Kolluru <[email protected]>

fixing lint

d72d7f1

Signed-off-by: Chaitanya Kolluru <[email protected]>

chaitanyakolluru force-pushed the helm-chart-proxy-rollout branch from 9bd4698 to d72d7f1 Compare October 1, 2025 15:57

dmvolod reviewed Oct 1, 2025

View reviewed changes

removes unneeded condition getter fn. replaces it with get from clust…

5cfd2d2

…er-api's condition package. Signed-off-by: Chaitanya Kolluru <[email protected]>

dmvolod reviewed Oct 1, 2025

View reviewed changes

chaitanyakolluru added 4 commits October 1, 2025 17:55

using conditions.IsFalse to determine condition status

f34e26f

Signed-off-by: Chaitanya Kolluru <[email protected]>

moved cluster detetion check to within the fn to reconcile for cluster

9807273

Signed-off-by: Chaitanya Kolluru <[email protected]>

sorts cluster meta by namespaced name before rolling out HelmReleaseP…

1f96a25

…roxies Signed-off-by: Chaitanya Kolluru <[email protected]>

rolloutStepSize description update

16d9383

Signed-off-by: Chaitanya Kolluru <[email protected]>

chaitanyakolluru requested a review from dmvolod October 6, 2025 01:51

dmvolod reviewed Oct 7, 2025

View reviewed changes

chaitanyakolluru requested a review from dmvolod October 7, 2025 17:13

dmvolod approved these changes Oct 7, 2025

View reviewed changes

jackfrancis reviewed Oct 10, 2025

View reviewed changes

verbose naming for getNamespacedNameStringFor

0d755be

Signed-off-by: Chaitanya Kolluru <[email protected]>

chaitanyakolluru requested a review from jackfrancis October 13, 2025 15:56

chaitanyakolluru added 2 commits October 15, 2025 19:07

adding hrp ready condition check

daeeb40

Signed-off-by: Chaitanya Kolluru <[email protected]>

ensure rollout completed condition is setup for rolled out hrp

4eb5438

Signed-off-by: Chaitanya Kolluru <[email protected]>

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 16, 2025

adding a check for hrp ready condition being unknown

2d32f4d

Signed-off-by: Chaitanya Kolluru <[email protected]>

chaitanyakolluru force-pushed the helm-chart-proxy-rollout branch from 87e97d1 to 2d32f4d Compare October 16, 2025 00:26

chaitanyakolluru requested a review from dmvolod October 16, 2025 13:35

Merge branch 'kubernetes-sigs:main' into helm-chart-proxy-rollout

8d0c3a3

chaitanyakolluru mentioned this pull request Nov 3, 2025

✨Rollout options added to HelmChartProxy #452

Draft

chaitanyakolluru closed this Nov 3, 2025

✨Add an option to control initial rollout of HelmReleaseProxies #443

✨Add an option to control initial rollout of HelmReleaseProxies #443

Uh oh!

Conversation

chaitanyakolluru commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 1, 2025

Uh oh!

k8s-ci-robot commented Oct 1, 2025

Uh oh!

mboersma commented Oct 1, 2025

Uh oh!

dmvolod Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaitanyakolluru Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Oct 7, 2025

Uh oh!

chaitanyakolluru commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaitanyakolluru commented Oct 16, 2025

Uh oh!

k8s-ci-robot commented Nov 3, 2025

Uh oh!

chaitanyakolluru commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chaitanyakolluru commented Oct 1, 2025 •

edited

Loading

linux-foundation-easycla bot commented Oct 1, 2025 •

edited

Loading

dmvolod Oct 1, 2025 •

edited

Loading

chaitanyakolluru Oct 7, 2025 •

edited

Loading